Skip to content

feat: Add cmake & ninja build requirements only when needed #2451

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
mgorny wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: Add cmake & ninja build requirements only when needed #2451

mgorny wants to merge 2 commits into from

Conversation

@mgorny
Copy link
Contributor

@mgorny mgorny commented Jan 17, 2025

Rather than listing cmake and ninja build requirements unconditionally, add them to setup_requires when they are necessary -- that is, when using skbuild and when the system installations of these tools are not available. This avoids installing the packages unnecessarily, and improves portability by using downstream patched CMake version when available.

@mgorny
feat: Add cmake & ninja build requirements only when needed
a25b204 
Rather than listing `cmake` and `ninja` build requirements
unconditionally, add them to `setup_requires` when they are necessary --
that is, when using skbuild and when the system installations of these
tools are not available.  This avoids installing the packages
unnecessarily, and improves portability by using downstream patched
CMake version when available.
@mgorny
Copy link
Contributor Author

mgorny commented May 21, 2025

Gentle ping.

@ddelange
Copy link
Contributor

ddelange commented May 21, 2025

@maartenbreddels can you please merge #2449 so CI goes green and this PR can pull latest master?

@ddelange
Copy link
Contributor

ddelange commented Sep 3, 2025

@mgorny master now contains the latest fixes for CI and numpy v2 support

@mgorny
Copy link
Contributor Author

mgorny commented Feb 4, 2026

Ah, sorry, I didn't realize I was expected to merge/rebase it against master.

@ddelange
Copy link
Contributor

ddelange commented Feb 4, 2026

@maartenbreddelsai flakey test failure, only on windows+3.12?

____________________ test_string_buffers[df_factory_numpy] ____________________

df_factory = <function df_factory_numpy.<locals>.create at 0x0000017F6A4F7A60>

    def test_string_buffers(df_factory):
        data = ["foo", "bar"]
        x = np.array(data, dtype="U8")
        df = df_factory(x=x)
        if isinstance(df["x"].values, pa.lib.ChunkedArray):
            pytest.xfail()
        interchange_df = df.__dataframe__()
        roundtrip_df = _from_dataframe_to_vaex(interchange_df)
>       assert roundtrip_df["x"].tolist() == data
               ^^^^^^^^^^^^^^^^^^^^^^^^^^

tests\dataframe_protocol_test.py:435: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
C:\hostedtoolcache\windows\Python\3.12.10\x64\Lib\site-packages\vaex\expression.py:806: in tolist
    return values.to_pylist()
           ^^^^^^^^^^^^^^^^^^
pyarrow\\array.pxi:1775: in pyarrow.lib.Array.to_pylist
    ???
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

>   ???
E   UnicodeDecodeError: 'utf-8' codec can't decode byte 0x80 in position 0: invalid start byte

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@ddelange ddelange ddelange approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mgorny @ddelange @maartenbreddels